feat(iam): refuse to cache delegations seen revoked by pyropy · Pull Request #177 · fil-forge/ingot

pyropy · 2026-09-22T17:12:59Z

Refuses to cache an authorize response that carries a delegation Ingot has seen revoked, so a Hilt write whose commit failed after publishing cannot leave stale grants cached until midnight. Such a request is authorized once and not cached.

KeyProofs remembers revoked CIDs until the next UTC midnight
The uncached authorize path with a request-scoped delegation store
adminAccount helper
Tests for the recorded set and the uncached response

🤖 Generated with Claude Code

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

pyropy · 2026-09-23T11:42:57Z

@codex review

chatgpt-codex-connector · 2026-09-23T11:43:09Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-01T14:11:23.444742Z	`eba04d0`	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3e9aeafd4f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

pyropy · 2026-09-28T16:51:36Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a8a4a9d81a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

pyropy · 2026-10-01T12:12:31Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7591ff9e8e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

pyropy · 2026-10-01T14:07:47Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eba04d0ab9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

alanshaw · 2026-10-12T09:44:13Z

+// sees it) or after (the store holds the CID and is dropped), never between —
+// where it would find the store without the CID, clear nothing, and leave the
+// revoked delegation cached until the horizon.
+func (k *KeyProofs) DepositUnlessRevoked(store *DelegationCache, dlgs ...ucan.Delegation) bool {


Perhaps we could call this proofs or proofChain, implying it is a proof chain not just a set of random delegations. It would make the behaviour easier to understand.

Suggested change

func (k *KeyProofs) DepositUnlessRevoked(store *DelegationCache, dlgs ...ucan.Delegation) bool {

func (k *KeyProofs) DepositUnlessRevoked(store *DelegationCache, proofChain ...ucan.Delegation) bool {

Can we just make DelegationCache revocation aware so that when you call Add it simply doesn't add them if they've been revoked?

Done in f146cf7. Each key's DelegationCache now holds the shared revoked set, and Add refuses a batch containing a revoked delegation: it adds none of it and returns false. DepositUnlessRevoked is gone; the service calls Add directly. Request-scoped stores are built without the set, so a request authorized from a response Hilt served after a failed commit can still retrieve. The race with InvalidateHolders stays closed: Add holds the cache lock from the check through the last insert, and Contains takes the same lock. Add's parameter is now proofs, per your naming point.

I have reverted the change done in f146cf7 (revert commit: 9423fe6) as I didn't like what Claude has done there.

I was trying to refactor the DelegationCache as you suggested, but that would require us to write each revocation CID for every key in the cache. I think the original proposal with a central revocation cache might be the most pragmatic solution.

Renamed in dbb1cfe: DepositUnlessRevoked and addAll take proofs, and the test fake's field follows. The pre-existing Deposit and DelegationCache.Add parameters are outside this PR's diff and keep their name.

Reinstated as a fresh commit, 1f42918, with the two tweaks from your message: NewDelegationCache(revocations *gocache.Cache) takes the shared revoked set (nil for a request-scoped store) instead of a second constructor, and a refusal renews the revocation only once it has used up half of revokedTTL, read with GetWithExpiration, so a read is not always a write. The set is one *gocache.Cache shared by reference, so there is no per-key write of revocation CIDs; the earlier revert reasoned otherwise.

A Hilt write publishes its revocations before it commits. When Swarf accepted them and the commit then failed, the revoked delegations stayed stored at Hilt, so Ingot dropped the key's cache on the revocation and refilled it from Hilt's next response, which carried the same delegations. The console's retry republished identical revoke invocations, which Swarf deduplicated, so the intended change waited for midnight. KeyProofs now remembers every revoked CID until the next UTC midnight plus clock skew, the horizon of everything cached from an authorize response, whether or not a store held the delegation. A response carrying a remembered CID authorizes the one request it answers and caches nothing: no signing key, tenant, action set or delegations. The request keeps a store of its own holding the response's delegations, so its onward retrieval still has a chain. The key returns to Hilt on every request until a committed write issues fresh delegations, whose random nonces give them new CIDs. A restart clears the set with the caches. Implements the error-recovery section of fil-one/RFC#30 (d2cd4a6). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

main's GetUserAccountForRequest now holds the local fast path's result in a variable named account, which shadowed the helper of the same name. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

The request path checked the revoked set and then added the delegations to the key's store as two separate steps. A revocation landing between them found the store without the CID, cleared nothing, and left the revoked delegation cached until the horizon. KeyProofs.Revoked is now DepositUnlessRevoked: the check and the add hold the same lock, so a revocation lands either before (the check sees it) or after (the store holds the CID and is dropped). A race test runs the two against each other. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

A leaf-only authorize response is completed from /s3/bucket/info, and those delegations went into the key's store with no revocation check: a revoked intermediate there was cached along with the signing key, tenant and action set. cacheProofs now deposits through DepositUnlessRevoked on the shared store and reports a refusal, which takes the existing uncached path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

A revocation names only the CID, and the delegation behind it may never expire (/s3/bucket/info chains carry no expiry and DelegationCache keeps them until restart). Forgetting the revocation at midnight let the next authorize response pair a fresh leaf with the same stale bucket-info delegation and repopulate the caches. The revoked set now keeps entries for the process lifetime. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Remembering every revoked CID until restart grew without bound. An entry only matters while Hilt still serves the revoked grant (a write whose commit failed), so it now lives revokedTTL (48h) and each refusal of a response carrying it renews it. A failed write whose grant goes unserved for longer than that is forgotten; that gap is noted in the code. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Per review: the revocation check moves from KeyProofs.DepositUnlessRevoked into DelegationCache.Add. A key's store (built by KeyProofs.For) shares the revoked set and refuses a batch naming any of it, adding nothing and reporting false; a request-scoped store has no set and adds everything. Add holds the cache lock from the check through the last insert, and Contains takes the same lock, so a revocation still lands either before the check or after the insert. Add's parameter is now proofs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This reverts commit f146cf7. The revoked set has to outlive the per-key caches: InvalidateHolders drops the cache holding a revoked CID, idle caches are evicted, and a revocation can arrive before any cache holds the CID. The check stays in KeyProofs.DepositUnlessRevoked, next to the set it reads and under the lock it shares with InvalidateHolders. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…a proof chain Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

The revocation check moves from KeyProofs.DepositUnlessRevoked into DelegationCache.Add. NewDelegationCache takes the revoked-CID set KeyProofs shares across every key's store; a store built with it refuses a batch naming any revoked CID, adding nothing and reporting false, and a request-scoped store (nil set) adds everything. Add holds the cache lock from the check through the last insert, and Contains takes the same lock, so a revocation still lands either before the check or after the insert. A refusal renews the revocation only once it has used up half its life, so a read is not always a write. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Sets every staging pin in `nodes/staging/eu-central-3/apps/versions.env` to what `nodes/dev/apps/versions.env` pins on `main`. Merging this is the promotion: the node picks it up on its next reconcile pass, waits for a safe proving window and restarts each service that changed. This pull request is rebuilt on every push to `main` and closed once staging matches dev, so edits to it are overwritten unless they are pushed to `bot/promote-staging` by hand. ## ingot [`sha256:4fdff18`](https://github.com/orgs/fil-forge/packages/container/ingot/1378629238) → [`sha256:d686c7d`](https://github.com/orgs/fil-forge/packages/container/ingot/1385513652) What fil-forge/ingot brings over staging's pin, [`6071feb..431ad37`](fil-forge/ingot@6071feb...431ad37): - fil-forge/ingot#177 ([66e9659](fil-forge/ingot@66e9659)) - fil-forge/ingot#178 ([5b3e0b6](fil-forge/ingot@5b3e0b6)) - fil-forge/ingot#194 ([1c47c14](fil-forge/ingot@1c47c14)) - fil-forge/ingot#259 ([431ad37](fil-forge/ingot@431ad37))

Sets every staging pin in `nodes/staging/eu-central-3/apps/versions.env` to what `nodes/dev/apps/versions.env` pins on `main`. Merging this is the promotion: the node picks it up on its next reconcile pass, waits for a safe proving window and restarts each service that changed. This pull request is rebuilt on every push to `main` and closed once staging matches dev, so edits to it are overwritten unless they are pushed to `bot/promote-staging` by hand. ## ingot [`sha256:4fdff18`](https://github.com/orgs/fil-forge/packages/container/ingot/1378629238) → [`sha256:e890d57`](https://github.com/orgs/fil-forge/packages/container/ingot/1385622164) What fil-forge/ingot brings over staging's pin, [`6071feb..8a9e399`](fil-forge/ingot@6071feb...8a9e399): - fil-forge/ingot#177 ([66e9659](fil-forge/ingot@66e9659)) - fil-forge/ingot#178 ([5b3e0b6](fil-forge/ingot@5b3e0b6)) - fil-forge/ingot#194 ([1c47c14](fil-forge/ingot@1c47c14)) - fil-forge/ingot#259 ([431ad37](fil-forge/ingot@431ad37)) - fil-forge/ingot#260 ([8a9e399](fil-forge/ingot@8a9e399))

Sets every staging pin in `nodes/staging/eu-central-3/apps/versions.env` to what `nodes/dev/apps/versions.env` pins on `main`. Merging this is the promotion: the node picks it up on its next reconcile pass, waits for a safe proving window and restarts each service that changed. This pull request is rebuilt on every push to `main` and closed once staging matches dev, so edits to it are overwritten unless they are pushed to `bot/promote-staging` by hand. ## ingot [`sha256:4fdff18`](https://github.com/orgs/fil-forge/packages/container/ingot/1378629238) → [`sha256:24f1dfd`](https://github.com/orgs/fil-forge/packages/container/ingot/1385707121) What fil-forge/ingot brings over staging's pin, [`6071feb..7fab0fb`](fil-forge/ingot@6071feb...7fab0fb): - fil-forge/ingot#177 ([66e9659](fil-forge/ingot@66e9659)) - fil-forge/ingot#178 ([5b3e0b6](fil-forge/ingot@5b3e0b6)) - fil-forge/ingot#194 ([1c47c14](fil-forge/ingot@1c47c14)) - fil-forge/ingot#259 ([431ad37](fil-forge/ingot@431ad37)) - fil-forge/ingot#260 ([8a9e399](fil-forge/ingot@8a9e399)) - fil-forge/ingot#258 ([7fab0fb](fil-forge/ingot@7fab0fb))

Sets every staging pin in `nodes/staging/eu-central-3/apps/versions.env` to what `nodes/dev/apps/versions.env` pins on `main`. Merging this is the promotion: the node picks it up on its next reconcile pass, waits for a safe proving window and restarts each service that changed. This pull request is rebuilt on every push to `main` and closed once staging matches dev, so edits to it are overwritten unless they are pushed to `bot/promote-staging` by hand. ## ingot [`sha256:4fdff18`](https://github.com/orgs/fil-forge/packages/container/ingot/1378629238) → [`sha256:aa89c75`](https://github.com/orgs/fil-forge/packages/container/ingot/1385968292) What fil-forge/ingot brings over staging's pin, [`6071feb..7b28323`](fil-forge/ingot@6071feb...7b28323): - fil-forge/ingot#177 ([66e9659](fil-forge/ingot@66e9659)) - fil-forge/ingot#178 ([5b3e0b6](fil-forge/ingot@5b3e0b6)) - fil-forge/ingot#194 ([1c47c14](fil-forge/ingot@1c47c14)) - fil-forge/ingot#259 ([431ad37](fil-forge/ingot@431ad37)) - fil-forge/ingot#260 ([8a9e399](fil-forge/ingot@8a9e399)) - fil-forge/ingot#258 ([7fab0fb](fil-forge/ingot@7fab0fb)) - fil-forge/ingot#257 ([8f9fc3f](fil-forge/ingot@8f9fc3f)) - fil-forge/ingot#261 ([7b28323](fil-forge/ingot@7b28323))

Sets every staging pin in `nodes/staging/eu-central-3/apps/versions.env` to what `nodes/dev/apps/versions.env` pins on `main`. Merging this is the promotion: the node picks it up on its next reconcile pass, waits for a safe proving window and restarts each service that changed. This pull request is rebuilt on every push to `main` and closed once staging matches dev, so edits to it are overwritten unless they are pushed to `bot/promote-staging` by hand. ## ingot [`sha256:4fdff18`](https://github.com/orgs/fil-forge/packages/container/ingot/1378629238) → [`sha256:f629535`](https://github.com/orgs/fil-forge/packages/container/ingot/1386342616) What fil-forge/ingot brings over staging's pin, [`6071feb..6331454`](fil-forge/ingot@6071feb...6331454): - fil-forge/ingot#177 ([66e9659](fil-forge/ingot@66e9659)) - fil-forge/ingot#178 ([5b3e0b6](fil-forge/ingot@5b3e0b6)) - fil-forge/ingot#194 ([1c47c14](fil-forge/ingot@1c47c14)) - fil-forge/ingot#259 ([431ad37](fil-forge/ingot@431ad37)) - fil-forge/ingot#260 ([8a9e399](fil-forge/ingot@8a9e399)) - fil-forge/ingot#258 ([7fab0fb](fil-forge/ingot@7fab0fb)) - fil-forge/ingot#257 ([8f9fc3f](fil-forge/ingot@8f9fc3f)) - fil-forge/ingot#261 ([7b28323](fil-forge/ingot@7b28323)) - fil-forge/ingot#263 ([6331454](fil-forge/ingot@6331454))

Sets every staging pin in `nodes/staging/eu-central-3/apps/versions.env` to what `nodes/dev/apps/versions.env` pins on `main`. Merging this is the promotion: the node picks it up on its next reconcile pass, waits for a safe proving window and restarts each service that changed. This pull request is rebuilt on every push to `main` and closed once staging matches dev, so edits to it are overwritten unless they are pushed to `bot/promote-staging` by hand. ## ingot [`sha256:4fdff18`](https://github.com/orgs/fil-forge/packages/container/ingot/1378629238) → [`sha256:f629535`](https://github.com/orgs/fil-forge/packages/container/ingot/1386342616) What fil-forge/ingot brings over staging's pin, [`6071feb..6331454`](fil-forge/ingot@6071feb...6331454): - fil-forge/ingot#177 ([66e9659](fil-forge/ingot@66e9659)) - fil-forge/ingot#178 ([5b3e0b6](fil-forge/ingot@5b3e0b6)) - fil-forge/ingot#194 ([1c47c14](fil-forge/ingot@1c47c14)) - fil-forge/ingot#259 ([431ad37](fil-forge/ingot@431ad37)) - fil-forge/ingot#260 ([8a9e399](fil-forge/ingot@8a9e399)) - fil-forge/ingot#258 ([7fab0fb](fil-forge/ingot@7fab0fb)) - fil-forge/ingot#257 ([8f9fc3f](fil-forge/ingot@8f9fc3f)) - fil-forge/ingot#261 ([7b28323](fil-forge/ingot@7b28323)) - fil-forge/ingot#263 ([6331454](fil-forge/ingot@6331454)) Co-authored-by: fil-forge-bot[bot] <318653112+fil-forge-bot[bot]@users.noreply.github.com>

Copilot AI lite review requested due to automatic review settings September 22, 2026 17:13

Copilot AI reviewed Sep 22, 2026

pyropy added this pull request to stack #179 September 22, 2026 17:13

chatgpt-codex-connector Bot reviewed Sep 23, 2026

View reviewed changes

Comment thread iam/service.go Outdated

pyropy force-pushed the srdjan/feat/iam-revoked-set branch from 3e9aeaf to e7ccdc6 Compare September 28, 2026 13:45

chatgpt-codex-connector Bot reviewed Sep 28, 2026

View reviewed changes

Comment thread iam/keyproofs.go

pyropy marked this pull request as draft September 28, 2026 17:05

pyropy force-pushed the srdjan/feat/iam-revoked-set branch 3 times, most recently from a8a4a9d to 7591ff9 Compare October 1, 2026 12:11

chatgpt-codex-connector Bot reviewed Oct 1, 2026

View reviewed changes

Comment thread iam/service.go Outdated

pyropy force-pushed the srdjan/feat/iam-revoked-set branch 3 times, most recently from ddff603 to eba04d0 Compare October 1, 2026 14:06

chatgpt-codex-connector Bot reviewed Oct 1, 2026

View reviewed changes

Comment thread iam/keyproofs.go Outdated

pyropy force-pushed the srdjan/feat/iam-revoked-set branch from 696bc5f to eb9160d Compare October 1, 2026 15:18

pyropy marked this pull request as ready for review October 2, 2026 11:02

pyropy requested a review from a team October 2, 2026 11:03

pyropy force-pushed the srdjan/feat/iam-revoked-set branch 2 times, most recently from 8977533 to e9edb8c Compare October 2, 2026 12:15

pyropy force-pushed the srdjan/feat/iam-revoked-set branch 3 times, most recently from 80ab5ce to 9952a3d Compare October 12, 2026 08:06

alanshaw requested changes Oct 12, 2026

View reviewed changes

pyropy force-pushed the srdjan/feat/iam-revoked-set branch from 9423fe6 to 4f380c0 Compare October 13, 2026 07:53

pyropy requested a review from alanshaw October 13, 2026 14:45

pyropy and others added 10 commits October 13, 2026 17:32

fix(iam): name the admin-account helper past main's account variable

07d2cc0

main's GetUserAccountForRequest now holds the local fast path's result in a variable named account, which shadowed the helper of the same name. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

chore(iam): drop the ponytail: prefix from two comments

a24e83a

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

refactor(iam): call the delegations a deposit takes proofs, they are …

361b042

…a proof chain Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

pyropy force-pushed the srdjan/feat/iam-revoked-set branch from dbb1cfe to 361b042 Compare October 13, 2026 15:32

alanshaw approved these changes Oct 14, 2026

View reviewed changes

chore: pin hilt main at 42dd070e and libforge main at 6d9e32d4

5982c2b

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

pyropy merged commit 66e9659 into main Oct 14, 2026
20 checks passed

pyropy deleted the srdjan/feat/iam-revoked-set branch October 14, 2026 11:39

fil-forge-bot Bot mentioned this pull request Oct 14, 2026

Promote dev's images to staging fil-forge/infra-nodes#157

Merged

	func (k KeyProofs) DepositUnlessRevoked(store DelegationCache, dlgs ...ucan.Delegation) bool {
	func (k KeyProofs) DepositUnlessRevoked(store DelegationCache, proofChain ...ucan.Delegation) bool {

Conversation

pyropy commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

pyropy commented Sep 23, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

pyropy commented Sep 28, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

pyropy commented Oct 1, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

pyropy commented Oct 1, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

alanshaw Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

alanshaw Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

pyropy Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

pyropy Oct 13, 2026

Choose a reason for hiding this comment

Uh oh!

pyropy Oct 13, 2026

Choose a reason for hiding this comment

Uh oh!

pyropy Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pyropy commented Sep 22, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Sep 23, 2026 •

edited

Loading